-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove AddToFileServer() #76
base: master
Are you sure you want to change the base?
Conversation
586d91b
to
6337fab
Compare
Is there an argument for why AddToFileServer() should be removed? It's a breaking change and also the ability to add files to the file server at different points in the build scripts is a useful feature. |
It is no longer needed, because an application can now simply build a CMake list with the files. It can manipulate the list any time. Details are fully unter application control, until eventually the final list is passed to |
The current way doesn't require an application to manage the list itself, it simply has to call the function each time it wants to add files. This is a common pattern that's all throughout CMake and is also followed by the camkes-tool CMake modules as well. As far as I know there's no strong reason why the existing implementation is insufficient. Maybe there are some complicated apps that want to do more complicated build time processing and mutate the list of files in the file server after they've been added but these apps would be free to manage their own list before declaring the fileserver at the end also. |
I can see you point and agree that it is convenient for applications. I can also see that having the API |
b5d6d67
to
747533d
Compare
1e9ccd2
to
ce225ba
Compare
b9db776
to
169268e
Compare
Turns out we can also remove the convenience wrapper |
09963be
to
2fbfe08
Compare
70450f3
to
7e67f41
Compare
9774027
to
c68079b
Compare
Signed-off-by: Axel Heider <[email protected]>
With the new interface of DefineCAmkESVMFileServer() there is not need to have AddToFileServer(). CMake lists with files can be built instead and then passed to DefineCAmkESVMFileServer(). Signed-off-by: Axel Heider <[email protected]>
Remove convenience wrapper, as it is no longer useful. Signed-off-by: Axel Heider <[email protected]>
This PR is on top of #31 and removes the CMake function
AddToFileServer()
. It is not longer needed becauseDefineCAmkESVMFileServer()
can take files and list of files as parameter now. Then CMake lists can be used to build the CPIO content.In seL4/camkes-vm-examples#37 all calls to
AddToFileServer()
are removed. But I assume there are 3rd party systems out there that also use this function. So, as an intermediate step, we could haveAddToFileServer()
print a deprecation warning for the next months.Test with: seL4/camkes-vm-examples#37, seL4/seL4_tools#151